Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(analyzer): improvements to reporting and circular $ref discovery #723

Merged
merged 6 commits into from
Dec 7, 2022

Conversation

erunion
Copy link
Member

@erunion erunion commented Dec 7, 2022

🧰 Changes

Finally, I've updated the circularRefs OpenAPI analyzer query to use these new APIs so it returns exactly what circular $refs exist within an API definition, and not all $refs that still remain in the API definition after dereferencing:

screen_shot_2022-12-07_at_1 32 37_pm

Footnotes

  1. What's the analyzer? It's the new JSONPath query system I've been building for the new openapi:uses command in rdme.

@erunion erunion added the enhancement New feature or request label Dec 7, 2022
*
* @private
*/
cb?: () => void;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like having this option here just for testing purposes but because I changed way we use @readme/json-schema-ref-parser to instantiate it with a class constructor it's really hard with Jest to spy on new $RefParser.dereference() now to ensure that it's only ever called once.

Adding this callback in here, and then calling it in the .then() below replicates this behavior.

// with the schema path (file path, url, whatever) that the schema exists in. Because
// we don't care about this information for this reporting mechanism, and only the
// $ref pointer, we're removing it.
return `#${pointer.split('#')[1]}`;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example pointer coming back from json-schema-ref-parser looks like /path/to/file.yaml#/components/schemas/name. We only need the latter part so that's what this is doing here.

@erunion erunion requested review from kanadgupta, a team, Dashron and julshotal and removed request for a team December 7, 2022 21:45
Copy link
Member

@kanadgupta kanadgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few small nits but otherwise LGTM!

src/index.ts Outdated Show resolved Hide resolved
src/analyzer/queries/readme.ts Outdated Show resolved Hide resolved
erunion and others added 2 commits December 7, 2022 14:01
@erunion erunion merged commit b5d6f21 into main Dec 7, 2022
@erunion erunion deleted the feat/analyzer-improvements branch December 7, 2022 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants